Skip to content

feat(node-core): Add integration disabling mechanism to prevent instrumentation conflicts #17972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

feat(node-core): Add integration disabling mechanism to prevent instrumentation conflicts #17972

wants to merge 10 commits into from

Conversation

@RulaKhaled
Copy link
Member

@RulaKhaled RulaKhaled commented Oct 20, 2025 •
edited
Loading

When using higher-level integrations that wrap underlying libraries, both the wrapper integration and the underlying library integration can instrument the same API calls, resulting in duplicate spans. This is particularly problematic for:

  • LangChain wrapping AI providers (OpenAI, Anthropic, Google GenAI)
  • Any future integrations that wrap other instrumented libraries

The disabled integrations mechanism can be used as follows:

// Any integration can disable others to prevent conflicts
import { disableIntegrations, isIntegrationDisabled } from '@sentry/node-core';

// In higher-level integration's setupOnce():
disableIntegrations('LowerLevelIntegration');

// In integration setupOnce() is not called for the disabled integrations

and is used in LangChain to auto disable OpenAI, Anthropic AI, and Google GenAI integrations in setupOnce().

@github-actions
Copy link
Contributor

github-actions bot commented Oct 20, 2025 •
edited
Loading

size-limit report 📦

Path Size % Change Change
@sentry/browser 24.6 kB +0.03% +7 B 🔺
@sentry/browser - with treeshaking flags 23.09 kB +0.04% +7 B 🔺
@sentry/browser (incl. Tracing) 41.23 kB +0.02% +5 B 🔺
@sentry/browser (incl. Tracing, Profiling) 45.5 kB +0.01% +3 B 🔺
@sentry/browser (incl. Tracing, Replay) 79.7 kB +0.01% +4 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 69.38 kB +0.01% +5 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 84.4 kB +0.01% +6 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 96.57 kB - -
@sentry/browser (incl. Feedback) 41.27 kB +0.01% +1 B 🔺
@sentry/browser (incl. sendFeedback) 29.27 kB +0.03% +8 B 🔺
@sentry/browser (incl. FeedbackAsync) 34.19 kB +0.01% +1 B 🔺
@sentry/react 26.3 kB +0.02% +3 B 🔺
@sentry/react (incl. Tracing) 43.21 kB +0.02% +6 B 🔺
@sentry/vue 29.08 kB +0.02% +4 B 🔺
@sentry/vue (incl. Tracing) 43 kB +0.02% +6 B 🔺
@sentry/svelte 24.61 kB +0.03% +7 B 🔺
CDN Bundle 26.89 kB +0.02% +3 B 🔺
CDN Bundle (incl. Tracing) 41.77 kB - -
CDN Bundle (incl. Tracing, Replay) 78.3 kB +0.01% +2 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 83.77 kB +0.01% +2 B 🔺
CDN Bundle - uncompressed 78.84 kB -0.01% -4 B 🔽
CDN Bundle (incl. Tracing) - uncompressed 123.96 kB -0.01% -4 B 🔽
CDN Bundle (incl. Tracing, Replay) - uncompressed 239.99 kB -0.01% -4 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 252.75 kB -0.01% -4 B 🔽
@sentry/nextjs (client) 45.31 kB +0.02% +7 B 🔺
@sentry/sveltekit (client) 41.61 kB +0.01% +4 B 🔺
@sentry/node-core 50.81 kB +0.11% +55 B 🔺
@sentry/node 157.98 kB +0.11% +166 B 🔺
@sentry/node - without tracing 92.69 kB +0.07% +56 B 🔺
@sentry/aws-serverless 106.4 kB +0.04% +33 B 🔺

View base workflow run

@github-actions
Copy link
Contributor

github-actions bot commented Oct 20, 2025 •
edited
Loading

node-overhead report 🧳

Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.

Scenario Requests/s % of Baseline Prev. Requests/s Change %
GET Baseline 9,136 - 9,103 +0%
GET With Sentry 1,400 15% 1,364 +3%
GET With Sentry (error only) 6,132 67% 6,074 +1%
POST Baseline 1,199 - 1,206 -1%
POST With Sentry 525 44% 511 +3%
POST With Sentry (error only) 1,072 89% 1,064 +1%
MYSQL Baseline 3,341 - 3,273 +2%
MYSQL With Sentry 485 15% 463 +5%
MYSQL With Sentry (error only) 2,737 82% 2,675 +2%

View base workflow run

@RulaKhaled RulaKhaled force-pushed the disable-integrations branch from fd71786 to 79b5d89 Compare October 22, 2025 14:50
@RulaKhaled RulaKhaled force-pushed the disable-integrations branch from 470034b to 31cce57 Compare October 23, 2025 09:14
@RulaKhaled RulaKhaled marked this pull request as ready for review October 23, 2025 09:16
@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Race Condition in AI Provider Setup

A race condition exists where the LangChain integration's setupOnce() method disables other AI provider integrations to prevent duplicate spans. However, if an AI provider's setupOnce() runs before LangChain's due to unpredictable integration setup order, it will instrument itself, leading to duplicate spans that this disabling mechanism intends to prevent.

Fix in Cursor Fix in Web

@RulaKhaled RulaKhaled marked this pull request as draft October 23, 2025 12:44
@RulaKhaled RulaKhaled changed the title feat(node-core): Add integration disabling mechanism to prevent instrumentation conflicts wip feat(node-core): Add integration disabling mechanism to prevent instrumentation conflicts Oct 23, 2025
@RulaKhaled RulaKhaled changed the title wip feat(node-core): Add integration disabling mechanism to prevent instrumentation conflicts feat(node-core): Add integration disabling mechanism to prevent instrumentation conflicts Nov 4, 2025
@RulaKhaled RulaKhaled marked this pull request as ready for review November 4, 2025 15:52
s1gr1d
s1gr1d reviewed Nov 5, 2025
View reviewed changes
Copy link
Member

@s1gr1d s1gr1d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I added some comments regarding the current naming of the functions as they kinda imply a different meaning to what they are actually doing and I think it would be good to give this another look when we expose those functions to our users.

And maybe it's also worth to add some tests

@RulaKhaled
Copy link
Member Author

RulaKhaled commented Nov 7, 2025

@s1gr1d Aside from resolving the comments mentioned above, I made a quick refactor to the PR:

Previously, we unregistered all disabled integrations even if their modules weren’t required at runtime (i.e., when the patch wasn’t fired), which broke AI instrumentations. We also couldn’t have handled marking an integration as disabled in the patch while handling disabling in the setup logic, because that would’ve been too late, the integration would’ve already been registered during setup.

To fix this, I moved the registration of disabled integrations into the patch, ensuring it only happens when the module is actually used. Disabled integrations are now unregistered in _patch(), which serves as a good middle ground for now since we don’t want to require customers to disable integrations manually.

Ideally, we’ll support disabling integrations earlier (at setup time), possibly via lazy loading? but that’s TBD and out of scope for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@s1gr1d s1gr1d s1gr1d left review comments

@seer-by-sentry seer-by-sentry[bot] seer-by-sentry[bot] left review comments

@cursor cursor[bot] cursor[bot] left review comments

@andreiborza andreiborza Awaiting requested review from andreiborza

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@RulaKhaled @s1gr1d